Skip to content

refactor(tool): Tool.Def.execute returns Effect, rename defineEffect → define#21961

Merged
kitlangton merged 9 commits intodevfrom
fix-skill
Apr 11, 2026
Merged

refactor(tool): Tool.Def.execute returns Effect, rename defineEffect → define#21961
kitlangton merged 9 commits intodevfrom
fix-skill

Conversation

@kitlangton
Copy link
Copy Markdown
Contributor

Summary

  • Change Tool.Def.execute return type from Promise to Effect.Effect
  • Rename Tool.defineEffectTool.define, delete old Promise-based Tool.define
  • Remove Effect.runPromise from all 18 tool execute boundaries
  • Update wrap() to use Effect.gen for validation and truncation
  • Convert prompt.ts dispatch to yield* item.execute() directly
  • Wrap plugin tool execute in Effect.promise in registry
  • Wire AbortController + Effect.onInterrupt for execRead abort propagation
  • Update all tool and session tests

Bash keeps ctx.abort + Effect.callback race for cancel support — the full output needs to go through normal truncation, which requires the tool to return a value. Moving to pure Effect interruption for bash requires taking tool dispatch from the AI SDK (forkChild), tracked separately.

Test plan

  • bun run typecheck clean
  • bun run test test/tool/ — 167 pass
  • bun run test test/session/prompt-effect.test.ts — 34 pass

…→ define

- Change Tool.Def.execute return type from Promise to Effect.Effect
- Rename Tool.defineEffect to Tool.define, delete old Promise-based Tool.define
- Remove Effect.runPromise from all 18 tool execute boundaries
- Update wrap() to use Effect.gen for validation and truncation
- Convert prompt.ts dispatch to yield* item.execute() directly
- Wrap plugin tool execute in Effect.promise in registry
- Wire AbortController + Effect.onInterrupt for execRead abort propagation
- Update all tool tests to use Effect.runPromise(tool.execute(...))
The bash tool needs ctx.abort to handle user cancellation gracefully —
returning partial output through normal truncation. Pure Effect
interruption can't produce a return value (effect-smol's Effect.exit
doesn't consume interrupts). Moving to Effect interruption for bash
requires taking tool dispatch from the AI SDK (forkChild), which is
a separate change.
The task tool passes ctx.abort to sub-sessions. Without a real signal,
bash in sub-sessions can't handle cancellation. Use AbortController +
onInterrupt to fire the signal when the task fiber is interrupted.
Remove Effect.promise wrappers around ctx.ask() in all tools — now
just yield* ctx.ask(...) directly. Update prompt.ts boundary to return
Effect from permission.ask (with orDie). Plugin tool context converts
back to Promise at the boundary. Update all test mocks.
…annel

wrap() in tool.ts already applies Effect.orDie. Tools whose execute
has typed errors (codesearch, webfetch, etc.) still need their own
orDie to satisfy the Def.execute type. Tools with never errors
(edit, multiedit, todo, lsp) had redundant orDie — removed.
Also merge double .pipe() chain in prompt.ts subtask handling.
Close over Truncate.Service in registry layer gen and use
truncate.output() directly instead of the static Truncate.output()
facade. Also convert fromPlugin execute from Effect.promise to
Effect.gen for cleaner service usage.
- FileTime.withLock now accepts () => Effect<T> instead of () => Promise<T>
- edit.ts withLock callback converted from async to Effect.gen, uses
  AppFileSystem.Service, Format.Service, and FileTime.Service directly
- edit/write/apply_patch tools use Bus.Service instead of Bus.publish facade
- write.ts uses Format.Service instead of Format.file facade
Bare Effect.runPromise/runFork calls used Effect's default logger,
causing raw pino-style log output to leak to the terminal during
subtask execution. Capture the current ServiceMap via Effect.services()
and use Effect.runPromiseWith/runForkWith so all sub-effects inherit
the app's logger and other services.
@kitlangton kitlangton merged commit c5fb628 into dev Apr 11, 2026
7 of 9 checks passed
@kitlangton kitlangton deleted the fix-skill branch April 11, 2026 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant